Add JavaScript asset proxy integration by ChristianPavilonis · Pull Request #742 · IABTechLab/trusted-server

ChristianPavilonis · 2026-05-28T19:24:20Z

Summary

Add the JS Asset Proxy engineering spec and implement the js_asset_proxy integration
Support exact configured first-party GET routes for enabled assets, exact <script src> rewriting, disabled assets, and blocked script removal
Proxy configured HTTPS origins with streaming passthrough, no EC forwarding, a small request-header allowlist, fixed User-Agent, and controlled downstream response headers
Gate response streaming on adapter support so unsupported runtimes use the buffered fallback
Pin proxied asset responses to application/javascript; charset=utf-8 with X-Content-Type-Options: nosniff
Generate disabled-by-default JS asset proxy candidates from ts audit

Verification

cargo fmt --all -- --check
cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare && cargo clippy-cloudflare-wasm && cargo clippy-spin-native && cargo clippy-spin-wasm
cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spin
./scripts/test-cli.sh
cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
cd crates/trusted-server-js/lib && npx vitest run
cd crates/trusted-server-js/lib && npm run format
cd docs && npm run format

aram356

Summary

Adds the JS Asset Proxy integration (config-driven first-party serving of exact third-party script URLs with enabled/disabled/blocked modes), stream_response plumbing through proxy_request, and ts audit generation of disabled asset-proxy candidates. The design follows the spec closely and the security defaults are right (request-header allowlist only, no EC/Cookie forwarding, Set-Cookie stripped, HTTPS-only origins, opaque generated paths). Blocking items: a guaranteed 502 on the Cloudflare adapter, a CI fmt failure, and merge conflicts with main.

Blocking

🔧 wrench

Cloudflare adapter rejects stream_response, so every enabled asset request 502s there: see inline comment (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:264)
CI cargo fmt fails: edition-2024 import ordering on three use lines; see inline comment (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:14)
Merge conflicts with main: GitHub reports the PR as CONFLICTING; git merge-tree shows conflicts in crates/trusted-server-core/src/config.rs, crates/trusted-server-core/src/integrations/mod.rs, and trusted-server.example.toml. All three are mechanical (registration list, validated-IDs list, sample config), but the branch needs a merge or rebase before landing.

Non-blocking

🤔 thinking

builders() ordering is load-bearing but undocumented (crates/trusted-server-core/src/integrations/mod.rs:289)
Path validation permits / (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:120)

♻️ refactor

Configured origin_url is never normalized, so non-canonical configs silently fail to match (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:244)
No test drives IntegrationProxy::handle() end-to-end (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:449)

🌱 seedling

Conditional revalidation never 304s at the edge; future allowlist additions would turn upstream 304 into 502 (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:477)
<link rel="preload" as="script"> hints for blocked/rewritten assets are untouched (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:496)
Audit dedup keys on the full URL including volatile query strings (crates/trusted-server-cli/src/commands/audit/mod.rs:489)

⛏ nitpick

headers.get(VARY) takes only the first of repeated headers (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:340)
Content-Length dropped on a passthrough body (crates/trusted-server-core/src/integrations/js_asset_proxy.rs:334)
#[cfg(test)] build_draft_config wrapper (crates/trusted-server-cli/src/commands/audit/mod.rs:331)

CI Status

fmt: FAIL (import ordering; reproduced locally)
clippy/check (all adapters): PASS
rust tests (fastly, axum, cloudflare, spin, CLI, parity, browser/integration): PASS
js tests (vitest): PASS
docs/ts format: PASS
mergeable: CONFLICTING

prk-Jr

Summary

The integration is well-shaped for the existing registry/proxy patterns, the header policy is tight, and the audit-side generator produces a safe disabled-by-default inventory with opaque randomized paths. Four items block: the required cargo fmt check is still red, the always-on stream_response flag is rejected outright by the Cloudflare and Spin adapters, the branch now conflicts with main, and any upstream redirect turns into a hard 502.

Note on overlap: the previous CHANGES_REQUESTED review is pinned to this exact head (6d6f5892) and no commits have landed since, so all of its threads are still open. This pass does not restate them — it confirms the two blocking ones (below) and adds what is new.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it as a commit on the PR branch. The remaining comments describe the fix in prose because the change is a design decision or spans multiple files and can't be auto-applied.

Blocking

🔧 wrench

Upstream 3xx becomes a hard 502 — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:477
cargo fmt required check is failing — see Cross-cutting below
Cloudflare and Spin adapters reject stream_response — see Cross-cutting below
Branch conflicts with main — see Cross-cutting below

Non-blocking

🤔 thinking / ♻️ refactor / ⛏ nitpick / 📝 note

Audit-generated drafts override upstream cache headers for every asset — see inline at crates/trusted-server-cli/src/commands/audit/mod.rs:444
Fixed User-Agent collapses UA-adaptive vendor bundles — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:281
Integration ID duplicated as a string literal — see inline at crates/trusted-server-core/src/config.rs:138
Only GET is registered for asset paths — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:445
X-TS-JS-Asset-Proxy marker is always emitted — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:350

Cross-cutting / body-level findings

🔧 cargo fmt required check is failing — reproduced locally at this head. Three use lines in js_asset_proxy.rs need edition-2024 import ordering: line 14 (http::{Method, Request, Response, StatusCode, header}), line 25 (crate::proxy::{ProxyRequestConfig, proxy_request}), and the test import at line 526 (crate::html_processor::{HtmlProcessorConfig, create_html_processor}). A single cargo fmt --all fixes all three. This is the only failing check and it is branch-protection required.
🔧 Cloudflare and Spin adapters reject stream_response, so every proxied asset returns 502 there — build_proxy_config unconditionally sets .with_stream_response() (js_asset_proxy.rs:263). Both adapters treat that flag as an unsupported contract and error out rather than degrade:
- crates/trusted-server-adapter-cloudflare/src/platform.rs:269 — "streaming response bodies are not supported on the Cloudflare Workers runtime"
- crates/trusted-server-adapter-spin/src/platform.rs:311 — "Spin outbound HTTP does not support streaming responses"
proxy_request surfaces that as an error, and handle() maps every error to 502 with X-TS-Error: js-asset-origin-unreachable — so on those runtimes every configured asset is a hard failure, and the response looks like an origin problem rather than an unsupported platform contract. The Cloudflare guard's own comment ("These fields are only set by asset routes, which are not routed to the Cloudflare adapter today") is no longer true, because core integration routes dispatch on every adapter. The Axum adapter has no guard and simply ignores the flag, so it buffers — a third behaviour. The spec's "No adapter entry-point changes are expected if the existing integration registry dispatch is sufficient" (2026-04-01-js-asset-proxy-design.md:297) needs revisiting.

CI does not catch this: the cross-adapter parity suite passes only because its fixture never enables js_asset_proxy. Whichever way this is resolved — gate stream_response on adapter capability, make the non-Fastly adapters buffer instead of erroring, or document the integration as Fastly-only and fail config validation elsewhere — a parity or per-adapter test that enables one asset would keep it from regressing.
🔧 The branch conflicts with main — GitHub reports CONFLICTING; git merge-tree origin/main <head> shows content conflicts in crates/trusted-server-core/src/config.rs, crates/trusted-server-core/src/integrations/mod.rs, and trusted-server.example.toml. Worth flagging the last one specifically: replace_js_asset_proxy_section (audit/mod.rs:567) searches the embedded example config for a literal [integrations.js_asset_proxy] header and returns a hard CLI error if it is missing. If that header is dropped or renamed while resolving the conflict, every ts audit run fails, not just this integration — the unit test at audit/mod.rs:997 is what guards it.
👍 Praise — a few things worth calling out: the upstream Set-Cookie is deliberately dropped and the request-header allowlist is genuinely minimal (build_proxy_config, verified by build_proxy_config_forwards_only_asset_header_allowlist); the audit generator emits opaque randomized /assets/<hex>.js paths from OsRng rather than mirroring vendor filenames; the precedence tests against the native GPT rewriter cover all three proxy modes; and validate_js_asset_proxy_config correctly plugs a real hole — IntegrationSettings::get_typed returns early for explicitly-disabled configs before calling validate(), so without this deploy-time check an invalid disabled inventory would ship unvalidated.

CI Status

cargo fmt: FAIL (required)
cargo test: PASS (required)
format-docs: PASS (required)
format-typescript: PASS (required)
cargo check (cloudflare native + wasm32-unknown-unknown): PASS
cargo check/build/test (spin native + wasm32-wasip1): PASS
cargo test (axum native): PASS
cargo test (cross-adapter parity): PASS
cargo test (ts CLI, native): PASS
integration tests: PASS
integration tests (Fastly EC lifecycle): PASS
prepare integration artifacts: PASS
browser integration tests: PASS
vitest: PASS
Analyze (rust): PASS
Analyze (actions): PASS
Analyze (javascript-typescript): PASS
CodeQL: PASS

aram356 · 2026-08-20T15:48:09Z

@ChristianPavilonis to resolve feedback

prk-Jr

Summary

Adds a js_asset_proxy integration serving explicitly configured third-party JavaScript from exact first-party paths, plus ts audit generation of disabled-by-default candidate entries. The core mechanics are sound: exact-match routing with no prefix or wildcard, follow_redirects = false, a three-header request allowlist with copy_request_headers = false and a fixed User-Agent, no EC forwarding, and a response rebuilt from scratch so Set-Cookie and every other upstream header outside a small allowlist are dropped. I specifically probed SSRF, request/response header leakage, cross-adapter streaming parity, and route shadowing, and found no defect in any of them. One security-hardening gap and three smaller items below.

2 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change spans more than one range in the file or needs an accompanying test change.

Blocking

🔧 wrench

Proxied third-party bytes are served from the publisher origin with no content-type protection — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:378-381

Non-blocking

♻️ refactor

ETag/Last-Modified advertised downstream but conditional requests never forwarded upstream — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:288-296

🤔 thinking

Cache-Control: public on a route that can also mint an EC cookie — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:415-420

⛏ nitpick

Example config's sample asset is proxy = "enabled", so one edit activates it — see inline at trusted-server.example.toml:124

Cross-cutting / body-level findings

📝 The verification commands in the PR description are not this workspace's gates. The body lists cargo clippy --workspace --all-targets --all-features -- -D warnings and cargo test --workspace. Per CLAUDE.md, a workspace-wide clippy trips the Cloudflare adapter's non-wasm32 guard; run against this head it exits 101 at crates/trusted-server-adapter-cloudflare/src/lib.rs:5, so it cannot have passed as written. No quality problem behind it — I ran the real target-matched gates against 8fc2477 and all pass: cargo fmt --all -- --check, all six clippy-* aliases, all four test-* aliases, and the cross-adapter parity suite. Please update the description to the CLAUDE.md gate list.
👍 The supports_streaming_responses() gating in proxy.rs fixes a latent cross-adapter break. On main, handle_asset_proxy_request set with_stream_response() unconditionally (proxy.rs:1196) — a contract both the Cloudflare (adapter-cloudflare/src/platform.rs:307) and Spin (adapter-spin/src/platform.rs:318) clients hard-reject. Gating it plus the buffered fallback (proxy.rs:1211-1228), covered by new tests at proxy.rs:4265 and proxy.rs:4334, is a real fix beyond this PR's stated scope. Worth calling out in the description since it changes shared proxy behaviour.

CI Status

cargo fmt: PASS (required)
cargo test: PASS (required)
format-typescript: PASS (required)
format-docs: PASS (required)
cargo test (axum native): PASS
cargo test (ts CLI, native): PASS
cargo test (cross-adapter parity): PASS
cargo check (cloudflare native + wasm32-unknown-unknown): PASS
cargo check/build/test (spin native + wasm32-wasip1): PASS
vitest: PASS
integration tests: PASS
integration tests (Fastly EC lifecycle): PASS
browser integration tests: PASS
prepare integration artifacts: PASS
CodeQL: PASS
Analyze (rust): PASS
Analyze (actions): PASS
Analyze (javascript-typescript): PASS (reported twice, from two workflow runs)

…xy-spec # Conflicts: # trusted-server.example.toml

prk-Jr

Summary

Focused, well-tested integration that follows the existing integration-proxy shape closely and matches its own spec. One blocking finding: the asset config accepts unknown keys, so a misspelled proxy key silently fails open from blocked to enabled. The rest are non-blocking cleanups, one of which is a verified defect in the ts audit draft generator.

3 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. The remaining comment describes the fix in prose because it needs a registry-side change and can't be auto-applied.

All three suggestions were verified in an isolated worktree individually and as a batch: cargo fmt --all -- --check, cargo clippy-fastly, trusted-server-core (2291 tests, native host), cargo test-axum, cargo test-cloudflare, cargo test-spin, trusted-server-cli (158 tests, native host), and the cross-adapter parity suite (13 tests) — all green, with no drift between the approved bytes and the post-verification tree.

Blocking

🔧 wrench

Asset config accepts unknown keys, so a typo'd proxy silently fails open to enabled — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:54

Non-blocking

♻️ refactor

replace_js_asset_proxy_section swallows the comment block documenting the next section — see inline at crates/trusted-server-cli/src/commands/audit/mod.rs:588-593
Example template ships an active cache_ttl_seconds = 3600, contradicting this PR's own documented safe default — see inline at trusted-server.example.toml:167

🤔 thinking

el.tag_name() allocates per element in four handlers whose only consumer never reads it — see inline at crates/trusted-server-core/src/html_processor.rs:522

Cross-cutting / body-level findings

🌱 Core named routes silently shadow configured asset paths — IntegrationRegistry route insertion catches integration-vs-integration collisions loudly, which is what the spec asks for (docs/superpowers/specs/2026-04-01-js-asset-proxy-design.md: "an asset path cannot silently shadow another integration endpoint"). But in crates/trusted-server-adapter-fastly/src/app.rs, named routes resolve before state.registry.has_route(&method, &path) in dispatch_fallback, so an asset path colliding with a core route (/auction, /page-bids, /first-party/proxy, /_ts/*) is silently shadowed with no config-time error — the rewritten <script src> just hits the core handler and the asset is never proxied. Low practical risk today because ts audit generates opaque /assets/<24-hex>.js paths, but hand-written config has no guard. Worth a follow-up issue rather than widening this PR.
👍 Streaming capability gate applied at every call site — supports_streaming_responses() is the right fix for a flag that Cloudflare and Spin hard-reject, and crates/trusted-server-core/src/proxy.rs gates it in proxy_request, send_asset_origin_request, and handle_asset_proxy_request rather than only at the new call site. The buffered-fallback assertions (recorded_stream_response_flags()) pin the behaviour on both branches.
👍 Response is rebuilt from an allowlist, not filtered — finalize_asset_response starts from Response::new(body) and copies only Content-Encoding / ETag / Last-Modified / Vary / Cache-Control, so upstream Set-Cookie, CORS grants, and Content-Type cannot escape by construction. Forcing application/javascript; charset=utf-8 plus nosniff closes the "configured upstream serves an HTML document from the publisher origin" hole cleanly.
👍 The ordering constraint is enforced by tests, not just a comment — the "must remain first" note on js_asset_proxy in builders() is backed by js_asset_proxy_rewriter_takes_precedence_over_native_rewriters, js_asset_proxy_blocking_takes_precedence_over_native_rewriters, and disabled_js_asset_proxy_candidate_allows_native_rewriters, all driving a real IntegrationRegistry. Reordering builders() breaks CI instead of silently changing rewrite precedence.
👍 Deploy validation covers disabled asset inventory — validate_js_asset_proxy_config runs regardless of enabled, so audit-generated candidates can't rot into invalid config that only explodes when an operator flips enabled = true. validate_rejects_invalid_disabled_js_asset_proxy_assets locks that in.

CI Status

Analyze (actions): PASS
Analyze (javascript-typescript): PASS
Analyze (javascript-typescript): PASS
Analyze (rust): PASS
CodeQL: PASS
browser integration tests: PASS
cargo check (cloudflare native + wasm32-unknown-unknown): PASS
cargo check/build/test (spin native + wasm32-wasip1): PASS
cargo fmt: PASS (required)
cargo test: PASS (required)
cargo test (axum native): PASS
cargo test (cross-adapter parity): PASS
cargo test (ts CLI, native): PASS
format-docs: PASS (required)
format-typescript: PASS (required)
integration tests: PASS
integration tests (Fastly EC lifecycle): PASS
prepare integration artifacts: PASS
vitest: PASS

prk-Jr

Summary

Verdict: APPROVE — no blocking findings. Everything below is non-blocking: questions about deliberate design choices, one test-coverage gap, and small consistency items. None of it needs to land before merge.

Adds a js_asset_proxy integration that serves explicitly configured third-party JavaScript from first-party paths, plus the stream_response plumbing and adapter capability gate it needs, and ts audit generation of disabled-by-default candidates. The design is tight: exact-match-only routing and rewriting, a three-mode (enabled/disabled/blocked) per-asset switch, a narrow request-header allowlist, and a response built from an allowlist rather than passed through. All 19 CI checks pass. Nothing here blocks merge — the findings below are questions about deliberate design choices, a test-coverage gap, and small consistency items.

2 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch) to apply them as commits on the PR branch. The remaining comments describe the concern in prose because the change spans multiple files or is a question rather than an edit.

Non-blocking

🤔 thinking

Conditional-request headers dropped; a 304 would map to 502 — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:516
Fixed User-Agent vs UA-differentiated vendor bundles (and integrity) — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:303
cache_ttl_seconds upgrades private/no-store upstreams to public — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:360
The streaming response path is flag-asserted, never exercised — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:1398

⛏ nitpick

Use the existing without_ec_id() builder — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:289
Header-name constants don't follow the constants.rs convention — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:32
Spec 2's "Related context" paths don't exist — see inline at docs/superpowers/specs/2026-06-22-ts-audit-js-asset-proxy-config-design.md:11

🌱 seedling

IntegrationAttributeContext gained a public field — see inline at crates/trusted-server-core/src/integrations/registry.rs:85
ts audit mints fresh random paths on every run — see inline at crates/trusted-server-cli/src/commands/audit/mod.rs:127

👍 praise

Content-type pinning + nosniff — see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:387
The streaming capability gate fixes a real pre-existing bug — see inline at crates/trusted-server-core/src/proxy.rs:1237
Injection-safe TOML generation in the audit draft — see inline at crates/trusted-server-cli/src/commands/audit/mod.rs:651

Verified and cleared

Recording these so the next reviewer doesn't re-derive them:

Rewriter ordering holds. builders() order is preserved into inner.html_rewriters, and rewrite_attribute chains Replace without short-circuiting, so placing js_asset_proxy first genuinely gives it precedence over the native rewriters. Two tests guard it.
Configured paths can't shadow built-in routes. Integration routes dispatch in the EdgeZero fallback, after the tsjs and built-in route arms, so a configured path cannot capture /_ts/admin or /static/tsjs=.
No EC cookie lands on a publicly cacheable asset. handle_proxy skips EC generation for non-navigation requests, and enforce_set_cookie_cache_privacy is a second net at response send.
The expect() calls in finalize_asset_response are unreachable. Every value handed to HeaderValue::from_str came back through HeaderValue::to_str().ok(), so it is already visible ASCII.
The new non-streaming branch in handle_asset_proxy_request is not redundant — it mirrors the pre-existing shape at proxy.rs:1054 on main.
The cargo test --workspace / cargo clippy --workspace lines in the spec's Verification block are pre-existing house style across 10+ documents in docs/superpowers/specs/, not something this PR introduced.

CI Status

cargo test: PASS (required)
cargo fmt: PASS (required)
format-docs: PASS (required)
format-typescript: PASS (required)
cargo test (axum native): PASS
cargo test (cross-adapter parity): PASS
cargo test (ts CLI, native): PASS
cargo check (cloudflare native + wasm32-unknown-unknown): PASS
cargo check/build/test (spin native + wasm32-wasip1): PASS
vitest: PASS
integration tests: PASS
browser integration tests: PASS
integration tests (Fastly EC lifecycle): PASS
prepare integration artifacts: PASS
CodeQL: PASS
Analyze (rust): PASS
Analyze (actions): PASS
Analyze (javascript-typescript): PASS

prk-Jr · 2026-09-03T04:20:37Z

+            }
+        };
+
+        if !response.status().is_success() {


🤔 thinking — Conditional-request headers are dropped, and a 304 would land here as a 502.

finalize_asset_response forwards ETag and Last-Modified downstream, but build_proxy_config forwards only Accept, Accept-Language, and Accept-Encoding — If-None-Match and If-Modified-Since never reach the origin. The browser therefore caches with validators it can present, but every revalidation costs a full origin fetch and a full downstream transfer, where loading the script directly from the vendor CDN would have yielded a 304. For a proxy whose selling point is first-party delivery of vendor JS, that is a real bandwidth and latency regression versus the un-proxied baseline.

There is also an interlock worth recording before someone revisits the allowlist: StatusCode::is_success() is 200–299, so if conditional forwarding is added later without touching this check, an upstream 304 becomes 502 X-TS-Error: js-asset-origin-status and the script silently stops loading on every revalidation.

If the narrow allowlist is deliberate MVP scope — the spec does pin it to those three headers — a line in docs/superpowers/specs/2026-04-01-js-asset-proxy-design.md saying revalidation is intentionally not supported yet would keep the next reader from treating it as an oversight.

prk-Jr · 2026-09-03T04:20:37Z

+            HEADER_USER_AGENT.clone(),
+            http::HeaderValue::from_static("TrustedServer/1.0"),


🤔 thinking — The fixed User-Agent will change which bytes some origins return.

Plenty of tag and SDK CDNs branch on User-Agent — modern versus ES5 bundles, polyfill services, and vendor-specific device detection. Pinning every upstream fetch to TrustedServer/1.0 means all clients receive whatever the origin serves an unrecognized agent, which is usually the most conservative fallback and occasionally a bot/blocked response.

Two follow-on effects worth confirming:

If the publisher's markup carries integrity on the script tag, the attribute rewriter replaces src but leaves the hash in place. When the UA-selected bytes differ from what the publisher hashed, SRI fails and the script never executes — a failure mode that only shows up for assets the operator flips to proxy = "enabled".

Because the UA is constant, the response deliberately does not Vary on it, which is correct — but it means an origin that does vary by UA has its variants collapsed into one cached entry.

The spec mandates the fixed UA, so this is a "confirm the tradeoff is understood" note rather than a defect. Documenting it next to the assets[].proxy field would help operators recognize the symptom.

prk-Jr · 2026-09-03T04:20:37Z

+    fn resolved_cache_ttl_seconds(&self, asset: &JsAssetProxyAsset) -> Option<u32> {
+        asset.cache_ttl_seconds.or(self.config.cache_ttl_seconds)
+    }


🤔 thinking — A configured TTL rewrites private / no-store upstreams into public.

When either cache_ttl_seconds is set, finalize_asset_response emits Cache-Control: public, max-age=<ttl> and discards whatever the origin said. That is the documented behaviour, and the default (preserve upstream) is the safe one — but the override has no floor. A vendor bundle carrying per-user content, which the origin correctly marks private, no-store, becomes shared-cacheable at the edge the moment an operator sets a TTL on it, and every subsequent visitor gets the first visitor's copy.

The operator-facing text does not hint at this. In trusted-server.example.toml the whole guidance is:

# Uncomment to override upstream cache headers for every asset below. # cache_ttl_seconds = 3600

Two options, in increasing order of cost:

Extend that comment to say the override replaces private / no-store and must only be used for assets the operator knows are identical for every visitor. Note this comment block is duplicated verbatim in three places — here in trusted-server.example.toml, in the ts audit generator's string literal in crates/trusted-server-cli/src/commands/audit/mod.rs, and in the exact-string assertion in crates/trusted-server-cli/src/commands/config/init.rs — so it is a synchronized three-file edit rather than a one-click suggestion.

Refuse the public upgrade when the upstream response is private or no-store, logging at warn instead. That is a behaviour change beyond this PR's scope, so it would want a follow-up issue rather than a late edit here.

prk-Jr · 2026-09-03T04:20:37Z

+                .await
+                .expect("should proxy streaming asset response");
+            assert_eq!(success.status(), StatusCode::OK);
+            assert_eq!(streaming_stub.recorded_stream_response_flags(), vec![true]);


🤔 thinking — This asserts the streaming flag, but the streaming body is never exercised.

StubHttpClient::send always builds its response from a Vec<u8>, so it yields EdgeBody::Once no matter what set_streaming_responses_supported(true) reports. This test therefore proves that stream_response: true reached the platform request, and nothing more.

What stays uncovered is the path that actually runs in production on Fastly: an EdgeBody::Stream flowing through finalize_asset_response's into_parts() → Response::new(body) reconstruction, out of handle_proxy, and into the EdgeBody::Stream(_) arm of send_edgezero_response in crates/trusted-server-adapter-fastly/src/main.rs:345. Header-only handling of a stream body is exactly where a regression would hide — the reconstruction drops the upstream Content-Length (correct for a stream, and worth keeping that way), and nothing in the suite would notice if that changed.

A stub variant that can yield a Body::Stream would close this, and it would pay for itself for the other integrations that adopt with_stream_response() later. Not a merge blocker — the flag assertion plus the Fastly adapter's own streaming tests cover the halves separately — but the seam between them is untested.

prk-Jr · 2026-09-03T04:20:37Z

+            .with_stream_response()
+            .without_forward_headers();
+        config.follow_redirects = false;
+        config.forward_ec_id = false;


⛏ nitpick — ProxyRequestConfig already has a builder for this.

without_ec_id() exists at crates/trusted-server-core/src/proxy.rs:424 and is what every other caller uses; setting the field directly bypasses it. (follow_redirects on the line above genuinely has no builder, so that one stays as-is.)

Suggested change

config.forward_ec_id = false;

config = config.without_ec_id();

Scratch-verified on this head: cargo fmt --all -- --check, cargo clippy-fastly, and cargo check-fastly && cargo check-axum && cargo check-cloudflare all pass with this applied, with no formatter drift.

prk-Jr · 2026-09-03T04:20:38Z

 #[derive(Debug)]
 pub struct IntegrationAttributeContext<'a> {
    pub attribute_name: &'a str,
+    pub element_name: &'a str,


🌱 seedling — Adding a public field here is a breaking change for anyone constructing this struct.

IntegrationAttributeContext is a pub struct with all-pub fields and no constructor, so every struct-literal construction outside this crate breaks on a new field. In-tree that cost 12 test-only edits across 8 integration modules in this PR, which is fine — but the same tax lands on any out-of-tree integration, and this is the second context field the rewriter API has needed.

Worth considering before the next one:

#[derive(Debug)] #[non_exhaustive] pub struct IntegrationAttributeContext<'a> { pub attribute_name: &'a str, pub element_name: &'a str, // … }

with a constructor for the required fields. That makes future context additions additive for external callers and turns the in-tree churn into a single call-site change. Follow-up material, not something to hold this PR for.

prk-Jr · 2026-09-03T04:20:38Z

+impl OpaqueAssetPathGenerator for RandomOpaqueAssetPathGenerator {
+    fn next_path(&mut self) -> String {
+        let mut bytes = [0_u8; 12];
+        rand::rngs::OsRng.fill_bytes(&mut bytes);
+        format!("/assets/{}.js", lowercase_hex(&bytes))


🌱 seedling — Re-running ts audit mints an entirely new path for every asset.

Drawing from OsRng per invocation is the right call for the first draft — the paths are opaque, unguessable, and carry no vendor or filename semantics, exactly as the spec asks. The cost shows up on the second run: an operator who has already curated trusted-server.toml and re-audits the same page gets a completely disjoint set of path values, so there is no way to diff "what's new on this page since last time" against what they already configured. Every candidate looks new.

A follow-up worth tracking: derive the opaque id deterministically from the normalized origin_url under a per-deployment salt, or add a merge mode that reads the existing config and only appends origins it doesn't already carry. Either keeps paths opaque while making re-audits reviewable. Explicitly out of scope for this PR — the spec's non-goals rule out interactive and merge workflows for the first pass.

prk-Jr · 2026-09-03T04:20:38Z

+        // Upstream bytes are served from the publisher origin, so the upstream
+        // cannot choose a document MIME type or opt into browser MIME sniffing.
+        finalized.headers_mut().insert(
+            header::CONTENT_TYPE,
+            http::HeaderValue::from_static(JS_ASSET_CONTENT_TYPE),
+        );
+        finalized.headers_mut().insert(


👍 praise — Pinning the content type instead of forwarding it is the right instinct, and the comment says why.

Discarding the upstream Content-Type and hard-setting application/javascript; charset=utf-8 plus X-Content-Type-Options: nosniff means a vendor origin that answers a soft-404 with an HTML error page — or gets compromised outright — hands the browser a JavaScript syntax error rather than a document rendered on the publisher's own origin. That closes the one genuinely dangerous capability this feature would otherwise grant an upstream, since these bytes are now same-origin with the publisher.

Combined with finalize_asset_response rebuilding the response from an allowlist rather than passing headers through, Set-Cookie and any upstream CORS grant are dropped structurally rather than by an easily-forgotten strip list. Good shape.

prk-Jr · 2026-09-03T04:20:38Z

+    let stream_response = services.http_client().supports_streaming_responses();
+    if stream_response {
+        platform_req = platform_req.with_stream_response();
+    }


👍 praise — This gate fixes a real pre-existing bug, not just groundwork for the new integration.

On main this call site sets with_stream_response() unconditionally. Both non-Fastly adapters that see asset traffic hard-reject that flag rather than degrading:

crates/trusted-server-adapter-cloudflare/src/platform.rs:307 — "streaming response bodies are not supported on the Cloudflare Workers runtime"

crates/trusted-server-adapter-spin/src/platform.rs:318 — "Spin outbound HTTP does not support streaming responses"

So the asset proxy path was returning a platform error on those runtimes, and the cross-adapter parity suite would not have caught it because the rejection is inside the platform HTTP client rather than in routing. Gating on supports_streaming_responses() — with the default false on the trait so a new adapter fails safe into buffering — is the correct fix, and mirroring the existing shape at proxy.rs:1054 keeps the two asset paths reading the same way.

prk-Jr · 2026-09-03T04:20:38Z

+fn sanitized_comment_value(value: &str) -> String {
+    value
+        .chars()
+        .map(|ch| if ch.is_control() { ' ' } else { ch })
+        .collect()
+}
+
+fn toml_quoted_string(value: &str) -> String {


👍 praise — The generated TOML is injection-safe by construction.

ts audit writes attacker-influenced data — third-party script URLs harvested from a page the operator does not control — straight into a config file, so this is the one spot in the CLI where escaping actually matters. toml_quoted_string handles backslash, quote, the three common escapes, and falls through to \uXXXX for anything else in the control range, and sanitized_comment_value collapses control characters so a crafted value cannot terminate a # comment and open a new table. Both are applied at every site that emits a value or a comment.

Worth noting the paths themselves are equally careful: is_valid_generated_asset_path re-validates the generator's output against the /assets/<lowercase-hex>.js shape and hard-errors rather than trusting it, so a future generator swap cannot quietly emit something the core-side path validator would reject at load time. Nice defensive layering for a code path most reviewers would wave through.

aram356 assigned ChristianPavilonis Jun 1, 2026

jevansnyc reviewed Jun 5, 2026

View reviewed changes

Comment thread docs/superpowers/specs/2026-04-01-js-asset-proxy-design.md

ChristianPavilonis commented Jun 5, 2026

View reviewed changes

aram356 requested review from aram356 and prk-Jr June 8, 2026 15:39

ChristianPavilonis changed the title ~~Simplify JS asset proxy spec~~ Add JavaScript asset proxy integration Jun 10, 2026

ChristianPavilonis linked an issue Jun 10, 2026 that may be closed by this pull request

JS Asset proxy implementation #762

Open

ChristianPavilonis marked this pull request as draft June 10, 2026 17:48

ChristianPavilonis force-pushed the simplify-js-asset-proxy-spec branch 2 times, most recently from 6b9389b to e0d6bf8 Compare June 22, 2026 21:20

ChristianPavilonis force-pushed the simplify-js-asset-proxy-spec branch 2 times, most recently from 8b56f22 to 753da1f Compare July 1, 2026 18:37

ChristianPavilonis changed the base branch from main to feature/ts-cli-audit July 1, 2026 19:38

Base automatically changed from feature/ts-cli-audit to main July 1, 2026 21:09

ChristianPavilonis force-pushed the simplify-js-asset-proxy-spec branch 2 times, most recently from 7730c4f to d79e84b Compare July 7, 2026 18:06

ChristianPavilonis marked this pull request as ready for review July 7, 2026 18:43

aram356 mentioned this pull request Jul 7, 2026

JS Asset Proxy engineering spec #605

Closed

3 tasks

ChristianPavilonis force-pushed the simplify-js-asset-proxy-spec branch from ee2a692 to 03dd7b8 Compare July 13, 2026 19:48

aram356 added this to the 202608 milestone Aug 13, 2026

aram356 requested changes Aug 18, 2026

View reviewed changes

prk-Jr requested changes Aug 19, 2026

View reviewed changes

ChristianPavilonis added 7 commits August 19, 2026 20:32

Simplify JS asset proxy spec

9e649fd

update spec

7a91cc6

Add JavaScript asset proxy integration

d343b56

Add audit-generated JS asset proxy config

c6460db

Fix JS asset proxy review findings

c615b2d

Fix JS asset proxy rebase compatibility

bb58be0

Address JavaScript asset proxy review feedback

a82aaf2

ChristianPavilonis force-pushed the simplify-js-asset-proxy-spec branch from 6d6f589 to a82aaf2 Compare August 20, 2026 13:49

Merge branch 'main' into simplify-js-asset-proxy-spec

8fc2477

prk-Jr requested changes Aug 21, 2026

View reviewed changes

Comment thread crates/trusted-server-core/src/integrations/js_asset_proxy.rs

Comment thread crates/trusted-server-core/src/integrations/js_asset_proxy.rs

Comment thread crates/trusted-server-core/src/integrations/js_asset_proxy.rs

Comment thread trusted-server.example.toml

ChristianPavilonis added 2 commits August 28, 2026 11:51

Merge remote-tracking branch 'origin/main' into simplify-js-asset-pro…

fab4adf

…xy-spec # Conflicts: # trusted-server.example.toml

Harden JavaScript asset proxy response MIME type

55521cd

ChristianPavilonis requested review from aram356 and prk-Jr August 28, 2026 17:24

Merge branch 'main' into simplify-js-asset-proxy-spec

5a1f0c2

aram356 modified the milestones: 202608, 202609 Aug 31, 2026

prk-Jr requested changes Sep 1, 2026

View reviewed changes

Comment thread crates/trusted-server-core/src/integrations/js_asset_proxy.rs

Comment thread crates/trusted-server-cli/src/commands/audit/mod.rs Outdated

Comment thread trusted-server.example.toml Outdated

Comment thread crates/trusted-server-core/src/html_processor.rs

ChristianPavilonis mentioned this pull request Sep 1, 2026

September release candidate (DO NOT MERGE) #1112

Draft

Address remaining JavaScript asset proxy review feedback

9f3f9c5

ChristianPavilonis requested a review from prk-Jr September 2, 2026 17:58

Merge branch 'main' into simplify-js-asset-proxy-spec

071ece5

prk-Jr approved these changes Sep 3, 2026

View reviewed changes

aram356 added a commit that referenced this pull request Sep 3, 2026

Merge updated PR #742 into rc/202609

1a06f0e

		HEADER_USER_AGENT.clone(),
		http::HeaderValue::from_static("TrustedServer/1.0"),

	config.forward_ec_id = false;
	config = config.without_ec_id();

Uh oh!

Conversation

ChristianPavilonis commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related

Verification

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

🤔 thinking

♻️ refactor

🌱 seedling

⛏ nitpick

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prk-Jr left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

🤔 thinking / ♻️ refactor / ⛏ nitpick / 📝 note

Cross-cutting / body-level findings

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aram356 commented Aug 20, 2026

Uh oh!

prk-Jr left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

♻️ refactor

🤔 thinking

⛏ nitpick

Cross-cutting / body-level findings

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

prk-Jr left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

ChristianPavilonis commented May 28, 2026 •

edited

Loading